fix: #642: don't use go-cmp for outputting diff #737

ramonpetgrave64 · 2024-01-17T16:28:51Z

Previously we used the go-cmp's Diff for displaying a human-friendly diff between two structs in an error message.

I had intended to do a json print of the structs and do a line-by-line diff. There is an internal library for calculating text diff, but I don't see any external functions that expose it to make it available for our use: https://pkg.go.dev/golang.org/x/tools/internal/diff

Instead, this we will simply display both structs in their own "actual" and "expected" sections. The user can use their other tools to find a human-friendly diff.

Previously we used the go-cmp's Diff for displaying a human-friendly diff between two structs in an error message. I had intended to do a json print of the structs and do a line-by-line diff. There is an internal library for calculating text diff, but I don't see any external functions that expose it: https://pkg.go.dev/golang.org/x/tools/internal/diff Instead, this we will simply display both structs in their own "actual" and "expected" sections. The user can use their other tools to find a human-friendly diff. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

laurentsimon

LGTM, thanks!

…ramework#737) Previously we used the go-cmp's Diff for displaying a human-friendly diff between two structs in an error message. I had intended to do a json print of the structs and do a line-by-line diff. There is an internal library for calculating text diff, but I don't see any external functions that expose it to make it available for our use: https://pkg.go.dev/golang.org/x/tools/internal/diff Instead, this we will simply display both structs in their own "actual" and "expected" sections. The user can use their other tools to find a human-friendly diff. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 and others added 2 commits January 17, 2024 16:14

Merge branch 'main' into ramonpetgrave64-no-go-cmp-in-non-test

c78838e

ramonpetgrave64 marked this pull request as ready for review January 17, 2024 16:30

ramonpetgrave64 requested review from laurentsimon and kpk47 as code owners January 17, 2024 16:30

ramonpetgrave64 changed the title ~~fixes: #642: don't use go-cmp for outputting diff~~ fix: #642: don't use go-cmp for outputting diff Jan 17, 2024

laurentsimon approved these changes Jan 17, 2024

View reviewed changes

laurentsimon merged commit ceaebee into slsa-framework:main Jan 17, 2024
17 of 18 checks passed

chenrui333 mentioned this pull request Mar 26, 2024

slsa-verifier 2.5.1 Homebrew/homebrew-core#167250

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: #642: don't use go-cmp for outputting diff #737

fix: #642: don't use go-cmp for outputting diff #737

ramonpetgrave64 commented Jan 17, 2024

laurentsimon left a comment

fix: #642: don't use go-cmp for outputting diff #737

fix: #642: don't use go-cmp for outputting diff #737

Conversation

ramonpetgrave64 commented Jan 17, 2024

laurentsimon left a comment

Choose a reason for hiding this comment